-
Notifications
You must be signed in to change notification settings - Fork 63
@W-20813610: Allow admins to enable OAuth but lock the site users can sign into to SITE_NAME #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a new OAUTH_LOCK_SITE configuration variable (default: true) to control whether users must sign in to the site specified in SITE_NAME when using OAuth authentication. This addresses issue #192 where users could inadvertently sign into a different site if they had an active Tableau session in their browser.
Key changes:
- New
OAUTH_LOCK_SITEconfig variable with default value oftrueto enforce site locking - Site validation logic in OAuth callback to verify users signed into the correct site
- URL manipulation in OAuth authorization to control site picker visibility based on lock setting
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| types/process-env.d.ts | Adds type definition for OAUTH_LOCK_SITE environment variable |
| src/config.ts | Implements lockSite configuration property with default value of true |
| src/config.test.ts | Adds test coverage for the new lockSite configuration option |
| src/server/oauth/callback.ts | Implements site validation logic and appropriate error handling for mismatched sites |
| src/server/oauth/authorize.ts | Adds getOAuthRedirectUrl helper function to control site picker behavior and sets redirected parameter when site is locked |
| src/scripts/createClaudeMcpBundleManifest.ts | Adds OAUTH_LOCK_SITE to the manifest configuration schema |
| docs/docs/configuration/mcp-config/oauth.md | Documents the new OAUTH_LOCK_SITE configuration option with clear usage guidelines |
| docs/src/css/custom.css | Minor CSS improvement for table cell vertical alignment |
| tests/oauth/testSetup.ts | Updates mock site name to mcp-test to align with test expectations |
| tests/oauth/authorizationCodeCallback.test.ts | Adds comprehensive test cases for site locking scenarios including enabled/disabled states and site mismatch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
These changes introduce a new config variable
OAUTH_LOCK_SITE(default: true) which ensures that when users connect their agent, they sign in to the site specified in theSITE_NAMEconfig variable.This addresses #192 where users are able to sign in to some other site if they already happened to have an active Tableau session in their browser when connecting their agent.
The one major gotcha is that on Server, when site locking is enabled and the user already has an active Tableau session in their browser for some other site, then they will get an error that says
Invalid request. Did you sign into the wrong site? Expected: [SITE_NAME]. The user will need to sign out in their browser and reattempt to connect their agent.Here is the behavior matrix I compiled when doing my testing. I did test against Tableau Cloud but the oddities can be ignored since OAuth can't currently be used against Tableau Cloud outside of local testing.